Skip to content

refactor(skills)!: remove the GitLab die() helper in favor of GitLabError - #2779

Merged
Jamie Kim (jkim323) merged 20 commits into
mainfrom
refactor/1555-gitlab-single-failure-mechanism
Sep 3, 2026
Merged

refactor(skills)!: remove the GitLab die() helper in favor of GitLabError#2779
Jamie Kim (jkim323) merged 20 commits into
mainfrom
refactor/1555-gitlab-single-failure-mechanism

Conversation

@jkim323

@jkim323 Jamie Kim (jkim323) commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Pull Request

Description

Removes the die() helper from the GitLab skill so GitLabError becomes the module's single failure mechanism, and adds a source contract so the boundary cannot drift again.

GitLabError is the module-wide typed failure base. GitLabAPIError(GitLabError) is the API-specific subclass used for transport failures; non-transport validation, configuration, and command errors remain GitLabError.

This closes the last open acceptance criterion of #1555. The other seven criteria — LOGGER, the _emit output sink, _emit_debug_traceback, typed API error classes, the source-contract tests, and the gitlab_token global removal — already landed in #2718 for both the Jira and GitLab skills. No Jira changes are needed.

Why this is not cosmetic

die() was already redaction-safe: it routed through _emit(), so the raw-stderr leak described in the issue was fixed by #2718. The remaining problem was an internal contract violation. The GitLabError docstring stated that die() survives only for "the argument-parsing and command-dispatch layer, where no value is promised" — yet die() was being called from four helpers that do promise return values:

Helper Returns
_auth_headers dict[str, str]
_required_oauth_client_id str
project str
_read_capped bytes

That is exactly the "silent None return" hazard the docstring warns against. The documented contract and the code disagreed, and nothing enforced the split.

What changed

  • All 61 die() call sites now raise GitLabError(message, exit_code), preserving each original message and exit-code constant. Sites inside except blocks chain with from exc.
  • die() is deleted; the now-unused NoReturn import is dropped.
  • GitLabError's docstring is rewritten to describe the single-mechanism contract.
  • 42 SystemExit test references are migrated across 7 files.
  • New source contract: no raise SystemExit, no sys.exit(...), and no die definition outside the __main__ guard — with one negative case per predicate.

Behavior

Exit codes are unchanged. main() already caught GitLabError, emitted once via _emit(), and returned exc.exit_code, so no new error handling was introduced.

Two deliberate consequences, both improvements:

  1. Library helpers now raise without emitting. Emission happens once, at the main() boundary, removing a double-emission path.
  2. SystemExit is a BaseException and previously bypassed main() entirely. These failures now flow through main(), so GITLAB_DEBUG=1 shows a redacted traceback for argument-validation failures that previously printed only a one-line error.

Related Issue(s)

Closes #1555

Type of Change

Code & Documentation:

  • Breaking change (fix or feature causing existing functionality to change)

Other:

  • Script/automation (.ps1, .sh, .py)

die() was a public module attribute, so its removal is technically breaking for any external caller importing gitlab.die. The CLI surface, exit codes, and output format are unchanged.

Testing

Test migration

The migration was not a uniform substitution. Sites fell into three kinds:

Kind Migration
Direct-call pytest.raises(gitlab.GitLabError), .code to .exit_code, and stderr assertions moved to str(exc_info.value) because converted helpers no longer emit
main() boundary pytest.raises removed entirely; assert gitlab.main() == <constant> plus emitted stderr
Already typed Untouched

No assertion was weakened to a bare pytest.raises(Exception). Every migrated site that checked an exit code still checks the same constant.

TestDie was removed. Its exact error: boom single-line assertion was not already covered elsewhere — test_emit_writes_exactly_one_stderr_line exercises _emit directly, and no test previously reached main()'s handler because SystemExit bypassed it. A replacement assertion was added to a converted main() test:

assert capsys.readouterr().err == "error: --fields is not valid with auth commands\n"

Validation status

Command Result
npm run lint:py Pass
npm run validate:skills Pass — 74 skills, 0 errors, 0 warnings
npm run test:py Blocked — environmental, not a code failure

npm run test:py could not execute locally. uv sync fails with os error 32 inside the OneDrive-synced workspace and os error 396 outside it, PyPI is unreachable with a TLS HandshakeFailure, and the existing skill virtualenvs hold a partially installed pytest. This predates the change and affects unrelated skills identically.

Because the suite could not run, acceptance criteria were verified directly against system Python: die is absent from the module, the source contract rejects all three synthetic violations and accepts the guarded sys.exit(main()), and no SystemExit reference remains outside the contract's own test fixtures.

Reviewers: please confirm the GitLab suite passes in CI before merging. That is the one gate not satisfied locally.

Checklist

Required Checks

  • Documentation is updated (if applicable) — n/a, no doc-visible behavior change
  • Files follow existing naming conventions
  • Changes are backwards compatible (if applicable) — see Type of Change
  • Tests added for new functionality (if applicable)

Required Local Checks

  • npm run validate:local
  • npm run spell-check
  • npm run lint:md-links

Security Considerations

  • This PR does not contain any sensitive or NDA information
  • Any new dependencies have been reviewed for security issues
  • Security-related scripts follow the principle of least privilege

No new dependencies. No change to _redact, _REDACT_KEYS, _REDACT_PATTERNS, the emit helpers, AuthContext, the OAuth modules, or the audit sink. The existing print-ownership, LOGGER.exception, and two-owner credentialed-egress contracts pass unmodified.

Additional Notes

The path:line references in #1555 are stale — the skills moved from .github/skills/{jira,gitlab}/{jira,gitlab}/ to .github/skills/project-planning/{jira,gitlab}/ and both files roughly tripled in size. Worth correcting the issue body on close.

The PR validation failure on Asset Docs Validation was due to the shared PowerShell module installer assuming PSGallery was already registered on the runner. The installer now self-registers PSGallery on demand, and the fix is covered by focused tests in scripts/tests/security/Install-PSModules.Tests.ps1.
Coverage follow-up for review thread 3850925767: the complete production-only Ubuntu run now passes 288 tests with 87.70% line coverage (1,062/1,211), 78.86% branch coverage (317/402), and 85.49% combined coverage. The terminal report publishes missing lines and partial branches for each runtime module: _gitlab_credentials.py 84%, _gitlab_oauth.py 79%, and gitlab.py 88%. The GitLab project now measures branches across scripts, reports missing lines, and enforces an 80% combined floor through the existing pytest CI lane. A focused regression also verifies typed GitLabError exit-code propagation, single redacted emission, and debug-traceback delegation at main().

- convert all 61 die() sites to raise GitLabError, preserving message and exit code
- delete die(); main() already emits once and returns exc.exit_code
- migrate 42 SystemExit test refs, splitting direct-call from main() boundary sites
- add a source contract forbidding SystemExit, sys.exit, and die outside __main__
- close the last open acceptance criterion of #1555

🧹 - Generated by Copilot
@jkim323
Jamie Kim (jkim323) requested a review from a team as a code owner August 25, 2026 01:46
@jkim323 Jamie Kim (jkim323) self-assigned this Aug 25, 2026
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Eval Execution

Status: Passed

  • Artifacts evaluated: 0
  • Specs run: 0
  • Assertions passed: 0
  • Assertions failed (blocking): 0
  • Assertions failed (advisory): 0
  • Failed specs (merge-blocking): 0

No changed AI artifacts required evaluation.

@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.14286% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.02%. Comparing base (f24f4de) to head (9e74a3c).

Files with missing lines Patch % Lines
...b/skills/project-planning/gitlab/scripts/gitlab.py 75.36% 17 Missing ⚠️
scripts/security/Install-PSModules.ps1 93.02% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2779      +/-   ##
==========================================
+ Coverage   83.10%   84.02%   +0.91%     
==========================================
  Files         183       96      -87     
  Lines       34150    11823   -22327     
  Branches       25      226     +201     
==========================================
- Hits        28380     9934   -18446     
+ Misses       5767     1815    -3952     
- Partials        3       74      +71     
Flag Coverage Δ
docusaurus 89.92% <ø> (ø)
pester 84.19% <93.02%> (+0.31%) ⬆️
pytest 81.87% <75.36%> (-0.87%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
scripts/security/Install-PSModules.ps1 94.18% <93.02%> (-2.25%) ⬇️
...b/skills/project-planning/gitlab/scripts/gitlab.py 85.43% <75.36%> (-1.97%) ⬇️

... and 95 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. The failure-mechanism migration is consistent, the GitLab suite and repository checks pass, and the breaking API removal is disclosed. I found three medium follow-ups: tighten the AST contract so non-equality and else exits cannot bypass it; prevent provider-controlled OAuth causes from escaping redaction for direct helper consumers; and reconcile issue #1555's GitLabAPIError wording with the GitLabError base type used here before closing the issue.

Approving, as comments need to be closed before merge, and I know you do that!

Comment thread .github/skills/project-planning/gitlab/tests/test_gitlab_helpers.py Outdated
Comment thread .github/skills/project-planning/gitlab/scripts/gitlab.py Outdated
Comment thread .github/skills/project-planning/gitlab/scripts/gitlab.py
Comment thread .github/skills/project-planning/gitlab/tests/test_gitlab_helpers.py
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Scorecard details
PackageVersionScoreDetails
npm/fast-uri 3.1.6 🟢 8.3
Details
CheckScoreReason
Dependency-Update-Tool🟢 10update tool detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Code-Review🟢 6Found 17/26 approved changesets -- score normalized to 6
Security-Policy🟢 10security policy file detected
Maintained🟢 1030 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 10
Binary-Artifacts🟢 10no binaries found in the repo
Packaging⚠️ -1packaging workflow not detected
Pinned-Dependencies🟢 5dependency not pinned by hash detected -- score normalized to 5
Token-Permissions🟢 10GitHub workflow tokens follow principle of least privilege
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Vulnerabilities🟢 100 existing vulnerabilities detected
SAST🟢 8SAST tool is not run on all commits -- score normalized to 8
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Fuzzing⚠️ 0project is not fuzzed
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
CI-Tests🟢 1022 out of 22 merged PRs checked by a CI test -- score normalized to 10
Contributors🟢 10project has 17 contributing companies or organizations
npm/qs 6.16.0 🟢 5.4
Details
CheckScoreReason
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Security-Policy🟢 10security policy file detected
Code-Review⚠️ 2Found 7/30 approved changesets -- score normalized to 2
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Maintained🟢 1018 commit(s) and 3 issue activity found in the last 90 days -- score normalized to 10
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
CII-Best-Practices🟢 5badge detected: Passing
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/side-channel 1.1.1 🟢 4.4
Details
CheckScoreReason
Maintained🟢 55 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 5
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Binary-Artifacts🟢 10no binaries found in the repo
Code-Review⚠️ 0Found 0/30 approved changesets -- score normalized to 0
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
SAST⚠️ 0no SAST tool detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
Security-Policy🟢 10security policy file detected
npm/side-channel-list 1.0.1 UnknownUnknown
npm/baseline-browser-mapping 2.11.20 UnknownUnknown
npm/browserslist 4.28.8 🟢 6.5
Details
CheckScoreReason
Packaging⚠️ -1packaging workflow not detected
Maintained🟢 1030 commit(s) and 2 issue activity found in the last 90 days -- score normalized to 10
Code-Review⚠️ 1Found 3/27 approved changesets -- score normalized to 1
Security-Policy🟢 10security policy file detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions🟢 9detected GitHub workflow tokens with excessive permissions
Pinned-Dependencies🟢 10all dependencies are pinned
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection🟢 3branch protection is not maximal on development and all release branches
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/caniuse-lite 1.0.30001810 🟢 4.6
Details
CheckScoreReason
Code-Review⚠️ 1Found 3/24 approved changesets -- score normalized to 1
Maintained🟢 1030 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 10
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Binary-Artifacts🟢 10no binaries found in the repo
Pinned-Dependencies🟢 10all dependencies are pinned
Security-Policy⚠️ 0security policy file not detected
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ 0branch protection not enabled on development/release branches
Fuzzing⚠️ 0project is not fuzzed
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/electron-to-chromium 1.5.420 UnknownUnknown
npm/fast-uri 3.1.6 🟢 8.3
Details
CheckScoreReason
Dependency-Update-Tool🟢 10update tool detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Code-Review🟢 6Found 17/26 approved changesets -- score normalized to 6
Security-Policy🟢 10security policy file detected
Maintained🟢 1030 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 10
Binary-Artifacts🟢 10no binaries found in the repo
Packaging⚠️ -1packaging workflow not detected
Pinned-Dependencies🟢 5dependency not pinned by hash detected -- score normalized to 5
Token-Permissions🟢 10GitHub workflow tokens follow principle of least privilege
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Vulnerabilities🟢 100 existing vulnerabilities detected
SAST🟢 8SAST tool is not run on all commits -- score normalized to 8
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Fuzzing⚠️ 0project is not fuzzed
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
CI-Tests🟢 1022 out of 22 merged PRs checked by a CI test -- score normalized to 10
Contributors🟢 10project has 17 contributing companies or organizations
npm/node-releases 2.0.54 🟢 4
Details
CheckScoreReason
Binary-Artifacts🟢 10no binaries found in the repo
Packaging⚠️ -1packaging workflow not detected
Code-Review⚠️ 0Found 0/30 approved changesets -- score normalized to 0
Maintained🟢 1018 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 10
Pinned-Dependencies🟢 3dependency not pinned by hash detected -- score normalized to 3
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
SAST⚠️ 0no SAST tool detected
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Security-Policy⚠️ 0security policy file not detected
License🟢 10license file detected
Fuzzing⚠️ 0project is not fuzzed
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ 0branch protection not enabled on development/release branches
npm/qs 6.16.0 🟢 5.4
Details
CheckScoreReason
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Security-Policy🟢 10security policy file detected
Code-Review⚠️ 2Found 7/30 approved changesets -- score normalized to 2
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Maintained🟢 1018 commit(s) and 3 issue activity found in the last 90 days -- score normalized to 10
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
CII-Best-Practices🟢 5badge detected: Passing
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/side-channel 1.1.1 🟢 4.4
Details
CheckScoreReason
Maintained🟢 55 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 5
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Binary-Artifacts🟢 10no binaries found in the repo
Code-Review⚠️ 0Found 0/30 approved changesets -- score normalized to 0
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
SAST⚠️ 0no SAST tool detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
Security-Policy🟢 10security policy file detected
npm/update-browserslist-db 1.3.2 🟢 5.5
Details
CheckScoreReason
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Maintained🟢 1018 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 10
Code-Review⚠️ 1Found 4/28 approved changesets -- score normalized to 1
Packaging⚠️ -1packaging workflow not detected
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions🟢 9detected GitHub workflow tokens with excessive permissions
Pinned-Dependencies🟢 10all dependencies are pinned
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Security-Policy⚠️ 0security policy file not detected
License🟢 10license file detected
Fuzzing⚠️ 0project is not fuzzed
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ 0branch protection not enabled on development/release branches
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/fast-uri 3.1.6 🟢 8.3
Details
CheckScoreReason
Dependency-Update-Tool🟢 10update tool detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Code-Review🟢 6Found 17/26 approved changesets -- score normalized to 6
Security-Policy🟢 10security policy file detected
Maintained🟢 1030 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 10
Binary-Artifacts🟢 10no binaries found in the repo
Packaging⚠️ -1packaging workflow not detected
Pinned-Dependencies🟢 5dependency not pinned by hash detected -- score normalized to 5
Token-Permissions🟢 10GitHub workflow tokens follow principle of least privilege
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Vulnerabilities🟢 100 existing vulnerabilities detected
SAST🟢 8SAST tool is not run on all commits -- score normalized to 8
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Fuzzing⚠️ 0project is not fuzzed
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
CI-Tests🟢 1022 out of 22 merged PRs checked by a CI test -- score normalized to 10
Contributors🟢 10project has 17 contributing companies or organizations
npm/qs 6.16.0 🟢 5.4
Details
CheckScoreReason
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Security-Policy🟢 10security policy file detected
Code-Review⚠️ 2Found 7/30 approved changesets -- score normalized to 2
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Maintained🟢 1018 commit(s) and 3 issue activity found in the last 90 days -- score normalized to 10
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
CII-Best-Practices🟢 5badge detected: Passing
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/fast-uri 3.1.6 🟢 8.3
Details
CheckScoreReason
Dependency-Update-Tool🟢 10update tool detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Code-Review🟢 6Found 17/26 approved changesets -- score normalized to 6
Security-Policy🟢 10security policy file detected
Maintained🟢 1030 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 10
Binary-Artifacts🟢 10no binaries found in the repo
Packaging⚠️ -1packaging workflow not detected
Pinned-Dependencies🟢 5dependency not pinned by hash detected -- score normalized to 5
Token-Permissions🟢 10GitHub workflow tokens follow principle of least privilege
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Vulnerabilities🟢 100 existing vulnerabilities detected
SAST🟢 8SAST tool is not run on all commits -- score normalized to 8
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Fuzzing⚠️ 0project is not fuzzed
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
CI-Tests🟢 1022 out of 22 merged PRs checked by a CI test -- score normalized to 10
Contributors🟢 10project has 17 contributing companies or organizations
npm/qs 6.16.0 🟢 5.4
Details
CheckScoreReason
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Security-Policy🟢 10security policy file detected
Code-Review⚠️ 2Found 7/30 approved changesets -- score normalized to 2
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Maintained🟢 1018 commit(s) and 3 issue activity found in the last 90 days -- score normalized to 10
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
CII-Best-Practices🟢 5badge detected: Passing
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/baseline-browser-mapping 2.11.20 UnknownUnknown
npm/browserslist 4.28.8 🟢 6.5
Details
CheckScoreReason
Packaging⚠️ -1packaging workflow not detected
Maintained🟢 1030 commit(s) and 2 issue activity found in the last 90 days -- score normalized to 10
Code-Review⚠️ 1Found 3/27 approved changesets -- score normalized to 1
Security-Policy🟢 10security policy file detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions🟢 9detected GitHub workflow tokens with excessive permissions
Pinned-Dependencies🟢 10all dependencies are pinned
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection🟢 3branch protection is not maximal on development and all release branches
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/caniuse-lite 1.0.30001810 🟢 4.6
Details
CheckScoreReason
Code-Review⚠️ 1Found 3/24 approved changesets -- score normalized to 1
Maintained🟢 1030 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 10
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Binary-Artifacts🟢 10no binaries found in the repo
Pinned-Dependencies🟢 10all dependencies are pinned
Security-Policy⚠️ 0security policy file not detected
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ 0branch protection not enabled on development/release branches
Fuzzing⚠️ 0project is not fuzzed
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/electron-to-chromium 1.5.420 UnknownUnknown
npm/fast-uri 3.1.6 🟢 8.3
Details
CheckScoreReason
Dependency-Update-Tool🟢 10update tool detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Code-Review🟢 6Found 17/26 approved changesets -- score normalized to 6
Security-Policy🟢 10security policy file detected
Maintained🟢 1030 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 10
Binary-Artifacts🟢 10no binaries found in the repo
Packaging⚠️ -1packaging workflow not detected
Pinned-Dependencies🟢 5dependency not pinned by hash detected -- score normalized to 5
Token-Permissions🟢 10GitHub workflow tokens follow principle of least privilege
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Vulnerabilities🟢 100 existing vulnerabilities detected
SAST🟢 8SAST tool is not run on all commits -- score normalized to 8
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Fuzzing⚠️ 0project is not fuzzed
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
CI-Tests🟢 1022 out of 22 merged PRs checked by a CI test -- score normalized to 10
Contributors🟢 10project has 17 contributing companies or organizations
npm/node-releases 2.0.54 🟢 4
Details
CheckScoreReason
Binary-Artifacts🟢 10no binaries found in the repo
Packaging⚠️ -1packaging workflow not detected
Code-Review⚠️ 0Found 0/30 approved changesets -- score normalized to 0
Maintained🟢 1018 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 10
Pinned-Dependencies🟢 3dependency not pinned by hash detected -- score normalized to 3
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
SAST⚠️ 0no SAST tool detected
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Security-Policy⚠️ 0security policy file not detected
License🟢 10license file detected
Fuzzing⚠️ 0project is not fuzzed
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ 0branch protection not enabled on development/release branches
npm/qs 6.16.0 🟢 5.4
Details
CheckScoreReason
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Security-Policy🟢 10security policy file detected
Code-Review⚠️ 2Found 7/30 approved changesets -- score normalized to 2
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Maintained🟢 1018 commit(s) and 3 issue activity found in the last 90 days -- score normalized to 10
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
CII-Best-Practices🟢 5badge detected: Passing
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
npm/side-channel 1.1.1 🟢 4.4
Details
CheckScoreReason
Maintained🟢 55 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 5
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Packaging⚠️ -1packaging workflow not detected
Binary-Artifacts🟢 10no binaries found in the repo
Code-Review⚠️ 0Found 0/30 approved changesets -- score normalized to 0
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
SAST⚠️ 0no SAST tool detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
Security-Policy🟢 10security policy file detected
npm/update-browserslist-db 1.3.2 🟢 5.5
Details
CheckScoreReason
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Maintained🟢 1018 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 10
Code-Review⚠️ 1Found 4/28 approved changesets -- score normalized to 1
Packaging⚠️ -1packaging workflow not detected
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions🟢 9detected GitHub workflow tokens with excessive permissions
Pinned-Dependencies🟢 10all dependencies are pinned
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Security-Policy⚠️ 0security policy file not detected
License🟢 10license file detected
Fuzzing⚠️ 0project is not fuzzed
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ 0branch protection not enabled on development/release branches
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0

Scanned Files

  • docs/docusaurus/package-lock.json
  • package-lock.json
  • scripts/extension/marketplace-publisher/package-lock.json

@github-actions github-actions Bot mentioned this pull request Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comprehensive review found one blocking defect and three material follow-ups. The GitLab typed-failure migration is coherent, centralizes emission correctly, preserves redaction, and passed focused execution. I am requesting changes because the newly added PSGallery recovery fails in the exact missing-repository condition it is intended to handle.

Findings:

  • High: PowerShellGet rejects custom registration under the reserved PSGallery name; use the -Default registration flow.
  • Medium: API response-validation failures use GitLabError instead of the API-specific GitLabAPIError.
  • Medium: The source contract misses aliased and qualified exit mechanisms.
  • Medium: InstallationPolicy Trusted persists beyond this installer invocation unless explicitly restored or avoided.

Line comments contain the reproduction details and suggested corrections. Additional non-blocking observations concern PR scope, test readability, PowerShell help and naming, repeated mocks, and documenting repository mutation as a side effect.

Validation included 198 focused GitLab tests, 29 focused PowerShell tests, Ruff lint and format checks, PSScriptAnalyzer across 253 files, and an implementation-level PowerShellGet probe confirming the blocking parameter-set behavior. No accessibility barriers were found in the changed surfaces.

Comment thread scripts/security/Install-PSModules.ps1 Outdated
Comment thread scripts/security/Install-PSModules.ps1 Outdated
Comment thread .github/skills/project-planning/gitlab/scripts/gitlab.py Outdated
Comment thread .github/skills/project-planning/gitlab/tests/test_gitlab_helpers.py Outdated
@jkim323
Jamie Kim (jkim323) force-pushed the refactor/1555-gitlab-single-failure-mechanism branch from 34a1712 to a2514e5 Compare August 31, 2026 20:07
@jkim323
Jamie Kim (jkim323) force-pushed the refactor/1555-gitlab-single-failure-mechanism branch 2 times, most recently from 63d6c4c to 502821e Compare September 1, 2026 15:54
@jkim323
Jamie Kim (jkim323) force-pushed the refactor/1555-gitlab-single-failure-mechanism branch from 502821e to 4770a11 Compare September 1, 2026 16:08
@jkim323

Copy link
Copy Markdown
Collaborator Author

Copilot The npm Security Audit check is failing on this PR. The failure originates from vulnerable transitive dependencies rather than the GitLab refactor.

Known findings include:

ajv > fast-uri

All four are fixed in the v3 release line by fast-uri 3.1.6. The repository currently pins fast-uri to vulnerable version 3.1.5.

qs

Both are fixed by qs 6.16.0. The root lockfile currently resolves vulnerable qs 6.15.2.

Please remediate every affected npm project in the repository, not only the first audit lane that fails:

  1. Inspect all dependency paths and lockfiles:

    npm ls ajv fast-uri qs
    find . -name package-lock.json -not -path '*/node_modules/*' -print
  2. Update every fast-uri override from 3.1.5 to at least 3.1.6, including the root project and docs/docusaurus.

  3. Resolve every installed copy of qs to at least 6.16.0, preferably by updating its direct parent; use a narrow npm override where an upstream dependency cannot yet select the patched version.

  4. Regenerate every affected package-lock.json, including:

    • package-lock.json
    • docs/docusaurus/package-lock.json
    • scripts/extension/marketplace-publisher/package-lock.json if its installed fast-uri or qs remains vulnerable
  5. Do not add these advisories to audit-ci.json, because patched versions are available.

  6. Do not stop after resolving the listed advisories. Run fresh audits and remediate any additional non-allowlisted vulnerability reported by the current npm advisory database.

Verify from clean installs:

npm ci
npm run audit:npm:root

npm ci --prefix docs/docusaurus
npm run audit:npm:docs

npm ci --prefix scripts/extension/marketplace-publisher
npm audit --prefix scripts/extension/marketplace-publisher

npm run audit:npm

The final dependency trees must contain no fast-uri version below the applicable patched release and no qs version below 6.16.0:

npm ls fast-uri qs --all
npm ls fast-uri qs --all --prefix docs/docusaurus
npm ls fast-uri qs --all --prefix scripts/extension/marketplace-publisher

Keep the change limited to npm manifests and generated lockfiles unless another file is strictly required.

Failing job: https://github.com/microsoft/hve-core/actions/runs/33702622528/job/100484971614?pr=2779

🔒 - Generated by Copilot

Co-authored-by: jkim323 <46459431+jkim323@users.noreply.github.com>

Copilot AI commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Copilot The npm Security Audit check is failing on this PR. The failure originates from vulnerable transitive dependencies rather than the GitLab refactor.

Known findings includ...

Addressed in 98bc8a0. All affected npm projects now resolve fast-uri 3.1.6 and qs 6.16.0; Docusaurus also pins browserslist 4.28.8 for the additional current advisory. All three clean installs, individual audits, the aggregate audit, dependency-tree checks, and public-feed validation pass with zero vulnerabilities. No audit allowlist changes were made.

- verify default registration before installation
- use and clean up a temporary canonical fallback
- cover success and failure cleanup paths

🔧 - Generated by Copilot
@jkim323
Jamie Kim (jkim323) merged commit 2be7fc3 into main Sep 3, 2026
128 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[security] Architectural: introduce central output sink + LOGGER + typed error class to jira/gitlab skills

5 participants